-
-
Notifications
You must be signed in to change notification settings - Fork 518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Taps often lost on Android real device + Fabric with header enabled #1975
Comments
To add some further findings: TextInput has the same issue - taps are sometimes ignored, the input does not get focus and no keyboard. If I use |
Same for me, you need to tap on the top half of the pressable component to trigger the event. I switched to |
Does some know what is causing this? |
similar issue we're seeing with native stack on react native |
Any solution found for this? |
Support new architecture. software-mansion/react-native-screens#1975
…ement (software-mansion#2292) ## Description > [!important] This PR aims to fix only pressables on screen components. This PR does not fix similar pressable issue with pressables in native header. That interaction will be fixed separately. Pressable elements work just fine until there's a gesture involved. On sensitive physical devices even a little movement during the press is treated as a gesture. When the `Pressable` element detects a gesture it calls [onResponderMove](https://github.com/facebook/react-native/blob/82795715aefba07ae9d79278ce3fd4d2e9a928f2/packages/react-native/Libraries/Pressability/Pressability.js#L484) which then checks wether the gesture happened within the element or went outside by comparing the touch coordinates with coordinates of the element using `_isTouchWithinResponderRegion`. The `responderRegion` is obtained from `_responderID` and happens to have unexpected values when the native header is present. It tuns out that the Y origin is slightly off. After some further investigation and comparison of coordinates it turned out that the height of the android status bar is not well calculated in various scenarios: <table> <td> `statusBarHidden: true` </td> <td> `statusBarTranslucent: true` </td> <td> `statusBarTranslucent: false` </td> </tr> <tr> <td> ![Screenshot_1723212300](https://github.com/user-attachments/assets/57e2f4a3-b002-4ca3-9519-45cfece860c4) </td> <td> ![Screenshot_1723212331](https://github.com/user-attachments/assets/bd46c8d1-8813-4fae-a8a9-0326193371d2) </td> <td> ![Screenshot_1723212382](https://github.com/user-attachments/assets/c7373437-524d-4a0f-951e-ce2689a4fe5c) </td> </tr> </table> The `calculateHeaderHeight` used for calculating the header and statusBar height seems to be the problem. Luckily, we don't have to calculate it by ourselves anymore, because the correct `t` value is provided in the `onLayout` function of the `Screen`. Thus we can get rid of the custom function. Another issue found: after navigating to another screen the offset is off again (exactly by 2x). It's caused by changes introduced in [this PR](software-mansion#2169), which was supposed to prevent content jumps, but doesn't work since RN `0.75` sadly. ![Screenshot_1723220034](https://github.com/user-attachments/assets/b0908c23-4667-4ccf-8e5e-5e7e11bca316) I found out that `FrameOriginCorrection` is not being unset when dimensions from JVM are received, while the `FrameHeightCorrection` is. After adding the missing unset for `FrameOriginCorrection` I rolled back to the commit with the mentioned PR merged and RN `0.74` and I can confirm it works. Fixes software-mansion#1975 ## Changes - removed `calculateHeaderHeight` function - added unset for `FrameOriginCorrection` when dimensions from JVM are received - added `Test1975.tsx` repro - moved code responsible for determining header height during the very first render from component descriptor's `adopt` method to shadow node `appendChild`. ## Test code and steps to reproduce `TestHeader`, `Test1975` ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes --------- Co-authored-by: alduzy <alduzy@gmail.com> Co-authored-by: Alex Duży <91994767+alduzy@users.noreply.github.com>
…ement (#2292) > [!important] This PR aims to fix only pressables on screen components. This PR does not fix similar pressable issue with pressables in native header. That interaction will be fixed separately. Pressable elements work just fine until there's a gesture involved. On sensitive physical devices even a little movement during the press is treated as a gesture. When the `Pressable` element detects a gesture it calls [onResponderMove](https://github.com/facebook/react-native/blob/82795715aefba07ae9d79278ce3fd4d2e9a928f2/packages/react-native/Libraries/Pressability/Pressability.js#L484) which then checks wether the gesture happened within the element or went outside by comparing the touch coordinates with coordinates of the element using `_isTouchWithinResponderRegion`. The `responderRegion` is obtained from `_responderID` and happens to have unexpected values when the native header is present. It tuns out that the Y origin is slightly off. After some further investigation and comparison of coordinates it turned out that the height of the android status bar is not well calculated in various scenarios: <table> <td> `statusBarHidden: true` </td> <td> `statusBarTranslucent: true` </td> <td> `statusBarTranslucent: false` </td> </tr> <tr> <td> ![Screenshot_1723212300](https://github.com/user-attachments/assets/57e2f4a3-b002-4ca3-9519-45cfece860c4) </td> <td> ![Screenshot_1723212331](https://github.com/user-attachments/assets/bd46c8d1-8813-4fae-a8a9-0326193371d2) </td> <td> ![Screenshot_1723212382](https://github.com/user-attachments/assets/c7373437-524d-4a0f-951e-ce2689a4fe5c) </td> </tr> </table> The `calculateHeaderHeight` used for calculating the header and statusBar height seems to be the problem. Luckily, we don't have to calculate it by ourselves anymore, because the correct `t` value is provided in the `onLayout` function of the `Screen`. Thus we can get rid of the custom function. Another issue found: after navigating to another screen the offset is off again (exactly by 2x). It's caused by changes introduced in [this PR](#2169), which was supposed to prevent content jumps, but doesn't work since RN `0.75` sadly. ![Screenshot_1723220034](https://github.com/user-attachments/assets/b0908c23-4667-4ccf-8e5e-5e7e11bca316) I found out that `FrameOriginCorrection` is not being unset when dimensions from JVM are received, while the `FrameHeightCorrection` is. After adding the missing unset for `FrameOriginCorrection` I rolled back to the commit with the mentioned PR merged and RN `0.74` and I can confirm it works. Fixes #1975 - removed `calculateHeaderHeight` function - added unset for `FrameOriginCorrection` when dimensions from JVM are received - added `Test1975.tsx` repro - moved code responsible for determining header height during the very first render from component descriptor's `adopt` method to shadow node `appendChild`. `TestHeader`, `Test1975` - [x] Included code example that can be used to test this change - [x] Ensured that CI passes --------- Co-authored-by: alduzy <alduzy@gmail.com> Co-authored-by: Alex Duży <91994767+alduzy@users.noreply.github.com> (cherry picked from commit 34c1ba8)
Description
On a real android device, with fabric enabled, taps are often lost when Header is shown on Native Stack.
On an Android emulator, iOS and even using a tool such as scrcpy taps are always registered, using real device and manually tapping is very inconsistent. Have seen across multiple phones.
Steps to reproduce
react-native-screens
repofalse
logs WILL print with every tapSnack or a link to a repository
https://snack.expo.dev/X06saJ53E
Screens version
main
React Native version
0.72.4
Platforms
Android
JavaScript runtime
Hermes
Workflow
React Native (without Expo)
Architecture
Fabric (New Architecture)
Build type
Debug mode
Device
Real device
Device model
Nokia TA-1234
Acknowledgements
Yes
The text was updated successfully, but these errors were encountered: